Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typedefs generation #229

Merged
merged 69 commits into from
Feb 17, 2024
Merged

typedefs generation #229

merged 69 commits into from
Feb 17, 2024

Conversation

gucio321
Copy link
Collaborator

@gucio321 gucio321 commented Nov 8, 2023

This pull requests contains the following changes:

  • remove current structs generator
  • generate structs (and other types) basing on typedefs_dict.json (instead of structs_and_enums.json)
  • upgrade some types to be more meaningful for go users (for example textureID became type TextureID uint32)

@gucio321
Copy link
Collaborator Author

@gucio321 gucio321 linked an issue Nov 14, 2023 that may be closed by this pull request
@gucio321
Copy link
Collaborator Author

@ptxmac
Copy link
Contributor

ptxmac commented Nov 21, 2023

  • upgrade some types to be more meaningful for go users (for example textureID became type TextureID uint32)

I don't see this change in the PR? It's still

type TextureID struct {
	Data unsafe.Pointer
}

which is very inconvenient to work with

@gucio321
Copy link
Collaborator Author

I don't see this change in the PR? It's still

yep, it is unsafe.Pointer but I thnk to make it uintptr instead
Would it be better?

@AllenDang
Copy link
Owner

I remember I choose unsafe.Pointer for a reason, like race tracing issue.

@gucio321
Copy link
Collaborator Author

Yeah but this makes my feel a bit crazy.. golang/go#64111

@ptxmac
Copy link
Contributor

ptxmac commented Nov 23, 2023

I don't see this change in the PR? It's still

yep, it is unsafe.Pointer but I thnk to make it uintptr instead Would it be better?

Yeah, that would work, right now I'm using something like this

		ptr := unsafe.Pointer(uintptr(b.Tex.GetRef()))
		ImageV(TextureID{Data: ptr}, // ...

@ptxmac
Copy link
Contributor

ptxmac commented Nov 23, 2023

I remember I choose unsafe.Pointer for a reason, like race tracing issue.

How would that work? 🤔

@gucio321
Copy link
Collaborator Author

changed to uintptr. If it compiles with -race I can merge this PR

@gucio321
Copy link
Collaborator Author

well it doesn't compile unfortunately...
And, since GO Developers refuse to cooperate unfortunatley (AGAIN)

We can either use uintptr and fail each -race build or leafe texture as unsafe.pointer and suffer from the issue above...

@gucio321
Copy link
Collaborator Author

currently it applies to cimgui functions and typedefs.
What is missing is the struct_accessor that will be reworked in the next commit.

Rason for that is golang/go#64467
@38dh91
Copy link

38dh91 commented Feb 3, 2024

@gucio321 awesome! Your PR fixed not only font builder, but also clipper too. Thx a lot!

@gucio321
Copy link
Collaborator Author

gucio321 commented Feb 4, 2024

Great to hear this! I deffinitely need to finish this and merge to master!

@gucio321
Copy link
Collaborator Author

its mostly done, now I'm getting "works for me" issue.
Gonna get back here tomorrow.

@gucio321
Copy link
Collaborator Author

I pushed some fixes. I hope this will work soon.

0 is more appropiate null value for uintptr_t
this file is deprecated and was replaced by gengo_typedefs.go
@gucio321 gucio321 merged commit 20e032b into AllenDang:main Feb 17, 2024
2 checks passed
@gucio321 gucio321 deleted the callbacks branch February 17, 2024 09:41
@ptxmac
Copy link
Contributor

ptxmac commented Feb 17, 2024

@gucio321 Thanks for the effort! This PR took a while to get done, but it fixed all the problems I had workarounds for!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug FontGlyphRangesBuilder
4 participants